fix(command-mode): drain process output concurrently to prevent TerminalService pipe deadlock#430
fix(command-mode): drain process output concurrently to prevent TerminalService pipe deadlock#430postoso wants to merge 2 commits into
Conversation
…nalService pipe deadlock
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3101ebd992
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async let pendingOutput = Task.detached { outputHandle.readDataToEndOfFile() }.value | ||
| async let pendingError = Task.detached { errorHandle.readDataToEndOfFile() }.value |
There was a problem hiding this comment.
When Command Mode runs a command that produces unbounded or very large output, such as yes or a verbose build that runs until the 30s timeout, these detached reads now keep the pipes drained while accumulating the entire stream in Data before returning. That removes the previous OS-pipe-buffer cap, so the app can allocate hundreds of MB/GB and be killed before the timeout; please keep draining but impose a maximum captured byte count/truncation policy.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3101ebd992
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async let pendingOutput = Task.detached { outputHandle.readDataToEndOfFile() }.value | ||
| async let pendingError = Task.detached { errorHandle.readDataToEndOfFile() }.value |
There was a problem hiding this comment.
When Command Mode runs a command that produces unbounded or very high-volume output, such as yes, tail -f, or dumping a large file, these detached readDataToEndOfFile() calls now keep draining and accumulating the entire stream in memory until the process exits or the 30s timeout kills it. Removing the pipe backpressure fixes the 64KB deadlock, but without a maximum retained byte count this can consume large amounts of app memory and then append/send an enormous JSON tool result; consider draining while retaining only a bounded prefix/suffix or terminating once a limit is exceeded.
Useful? React with 👍 / 👎.
1 participant
Description
TerminalService.execute()used by Command Mode previously waited for the child process to exit before draining stdout and stderr. A command that wrote more than the pipe buffer could block onwrite()while FluidVoice blocked inwaitUntilExit(), so the command timed out and returned truncated output.This keeps the concurrent stdout/stderr drain from the original PR, but bounds how much output FluidVoice retains. Each stream is drained in chunks so the child process can finish, while FluidVoice captures at most 1 MiB per stream and appends an inline truncation notice when excess output is discarded.
The result shape is unchanged. Large finite commands can complete without pipe deadlock, and unbounded commands still respect the existing timeout without accumulating unlimited stdout/stderr in memory.
Type of Change
Related Issue or Discussion
Related to Command Mode terminal execution reliability. This updates #430 to address the Codex review finding about unbounded captured output while retaining the pipe-deadlock fix.
Testing
swiftlint lint --strict --config .swiftlint.yml Sources/Fluid/Services/TerminalService.swiftswiftlint lint --strict --config .swiftlint.yml Tests/FluidDictationIntegrationTests/DictationE2ETests.swiftswiftformat --config .swiftformat Sourcesxcodebuild test -project Fluid.xcodeproj -scheme Fluid -destination 'platform=macOS' CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED=NOAdded/updated TerminalService coverage for:
Notes
The capture cap is per stream. FluidVoice still drains stdout and stderr after the retained output reaches the cap; it just discards excess bytes so the child process does not block on a full pipe.
Screenshots / Video